fix: Honor EnumMember attribute in query-string serialization#241
fix: Honor EnumMember attribute in query-string serialization#241gjtorikian merged 3 commits intomainfrom
Conversation
The query-string builder used Enum.ToString(), producing
PascalCase values like "Desc" that the API rejects. It now
reads [EnumMember(Value="...")] to emit the correct wire
values ("desc", "asc").
Also makes ListOptions.Order nullable so callers can omit
the parameter entirely instead of always sending a value.
The test still referenced the old DeleteResourceByExternalId method signature, which was removed in 4ceb3de. Updated to use the current DeleteResource API so the test project compiles.
Greptile SummaryThis PR fixes query-string enum serialization by introducing Confidence Score: 5/5This PR is safe to merge — all three changes are targeted, backward-compatible, and well-scoped. No P0 or P1 issues found. The No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Enum property in options] --> B{value is null?}
B -- Yes --> C[Skip property - omit from query]
B -- No --> D[AddScalar switch on type]
D --> E[case Enum: ResolveEnumWireValue]
E --> F[GetField by name]
F --> G{Field found?}
G -- No --> H[Fallback: ToString]
G -- Yes --> I{EnumMember attr present?}
I -- No --> H
I -- Yes --> J[Return attr.Value]
J --> K[Append to query string]
H --> K
Reviews (2): Last reviewed commit: "fix: Restore default desc ordering for l..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| private static string ResolveEnumWireValue(Enum value) | ||
| { | ||
| var member = value.GetType().GetField(value.ToString()); | ||
| if (member != null) | ||
| { | ||
| var attr = member.GetCustomAttribute<EnumMemberAttribute>(); | ||
| if (attr != null && attr.Value != null) | ||
| { | ||
| return attr.Value; | ||
| } | ||
| } | ||
|
|
||
| return value.ToString(); | ||
| } |
There was a problem hiding this comment.
[Flags] enum combinations will miss the EnumMember lookup
value.GetType().GetField(value.ToString()) works correctly for simple enums, but for a [Flags] enum holding a combined value (e.g., A | B), ToString() produces "A, B" — a string that will never match any single field name, so GetField returns null and the method falls back to ToString(). No current PaginationOrder-style enum in this SDK uses [Flags], but this edge case is silently broken if any is added in the future. A defensive comment noting the limitation would prevent confusion.
Keep Order nullable (callers can set null to omit it) but default to PaginationOrder.Desc so existing callers that don't set Order explicitly still get order=desc on the wire.
1 participant
Summary
[EnumMember(Value="...")]instead of falling back toEnum.ToString(), so enum values likePaginationOrder.Descserialize asdesc(notDesc) on the wireListOptions.Orderis nowPaginationOrder?(nullable) so callers can omit theorderquery parameter entirelyNonGetQueryParamsTestcaused by theDeleteResourceByExternalId→DeleteResourcerenameTest plan
dotnet buildsucceeds (0 errors, 0 warnings)dotnet testpasses all 437 testsOrganizationsService.ListAsyncno longer returns a validation error for theorderparameter🤖 Generated with Claude Code